fixed favouriting feature with new packet structure#249
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
promatty
left a comment
promatty
left a comment
There was a problem hiding this comment.
the values in the favourites bar dont match what is displayed in the tabs
add.to.favs.mov
alexwhelan12
force-pushed
the
TEL-293-Fix-favourites-feature
branch
from
bbd3c47 to
1e579de
Compare
📝 WalkthroughWalkthroughThis pull request refactors favorite item tracking across PIS data components by introducing hierarchical path tracking through nested component trees, removing explicit fault flags from data definitions, and reworking the favorite lookup table logic to support array-based field extraction alongside recursive object traversal. A new dependency is also added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PISTransformer as PISTransformer<br/>(path=[])
participant FieldsPrinter as FieldsPrinter<br/>(basePath)
participant FieldPrinter as FieldPrinter<br/>(fieldPath)
participant useFavLookup as useFavouriteLookupTable
participant Favorites as Favorite Storage
User->>PISTransformer: Render PIS data with path
PISTransformer->>FieldsPrinter: Pass basePath=key (for arrays)
FieldsPrinter->>FieldPrinter: Propagate fieldPath=`${basePath}.${field.name}`
FieldPrinter->>useFavLookup: Query favorite using fieldPath
useFavLookup->>useFavLookup: extractFields: Traverse to leaf values
useFavLookup-->>FieldPrinter: Return formatted favorite label
FieldPrinter->>FieldPrinter: Display with formatPathLabel
FieldPrinter->>Favorites: Add/Remove favorite on user action
Favorites-->>FieldPrinter: Confirm favorite toggled
User-->>User: See updated favorite status
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 23: Remove the platform-specific SWC package entry
"@next/swc-darwin-arm64": "15.3.4" from package.json (do not replace it with
another platform-specific package); after removing, run your package manager
install (yarn/npm) to update the lockfile and ensure Next.js installs the
correct platform SWC binary automatically, and commit the updated package.json
and lockfile changes.
🧹 Nitpick comments (3)
packages/client/src/hooks/favouriteLookupTable.ts (1)
15-21: Add guard for empty arrays in I_PISField detection.The duck-typing check examines only
node[0]to determine if the array containsI_PISFieldobjects. Whilenode.lengthguards against empty arrays, consider explicitly handling arrays with mixed types or non-I_PISField contents more defensively.Optional: Add runtime type guard
if ( Array.isArray(node) && node.length && typeof node[0] === "object" && + node[0] !== null && "name" in node[0] && "data" in node[0] ) {packages/client/src/components/containers/BottomInformationContainer.tsx (2)
18-29: Simplify callback dependency array.
currentAppStateis listed as a dependency but isn't directly used in the callback body—onlyfavouritesandsetCurrentAppStateare referenced. This can cause unnecessary callback recreation when unrelated app state changes.Suggested fix
}, - [currentAppState, favourites], + [favourites, setCurrentAppState], );
31-41: Consider extracting shared formatting logic.This
formatPathLabelfunction applies the same regex transformations asformatKeyinPISTransformer.tsx(lines 189-194). Consider extracting this into a shared utility to avoid duplication.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
package.jsonpackages/client/src/components/containers/BottomInformationContainer.tsxpackages/client/src/components/transformers/PISTransformer.tsxpackages/client/src/hooks/PIS/PIS.battery.tsxpackages/client/src/hooks/PIS/PIS.faults.tsxpackages/client/src/hooks/favouriteLookupTable.ts
💤 Files with no reviewable changes (2)
- packages/client/src/hooks/PIS/PIS.battery.tsx
- packages/client/src/hooks/PIS/PIS.faults.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
packages/client/src/components/containers/BottomInformationContainer.tsx (1)
packages/client/src/hooks/PIS/PIS.mbms.tsx (1)
I_PIS(6-529)
packages/client/src/hooks/favouriteLookupTable.ts (2)
packages/client/src/objects/PIS/PIS.interface.ts (1)
I_PISField(44-44)packages/shared/src/functions.ts (1)
flattenObject(487-510)
packages/client/src/components/transformers/PISTransformer.tsx (1)
packages/client/src/contexts/AppStateContext.tsx (1)
useAppState(229-231)
🔇 Additional comments (5)
packages/client/src/hooks/favouriteLookupTable.ts (1)
22-31: LGTM!The field iteration correctly captures each
fieldin its own closure scope, handles missing data with optional chaining, and the boolean-to-string conversion is consistent with the display logic inRangeCheckedFieldData.packages/client/src/components/containers/BottomInformationContainer.tsx (1)
10-14: LGTM!Adding
mbmsto the data array properly extends favourite tracking to MBMS-related fields, supporting the new packet structure.packages/client/src/components/transformers/PISTransformer.tsx (3)
89-115: LGTM on path-based favourites tracking!The
fieldPathprop correctly enables hierarchical field identification for favourites. The callback properly usesfieldPathand the dependency array is accurate.
147-178: Path propagation looks correct.
FieldsPrintercorrectly constructsfieldPathfrombasePathandfield.name, maintaining consistency with the lookup table's path format.
180-227: Path tracking through recursive traversal is well-implemented.The
patharray correctly accumulates keys during traversal, andnewPath.join(".")produces paths that match the lookup table format. The recursive call toPISTransformerpreserves path context for nested structures.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@alexwhelan12 The green button is calling your name |
3 participants
Summary by CodeRabbit
Chores
New Features
✏️ Tip: You can customize this high-level summary in your review settings.